[SPARK-12424][ML] The implementation of ParamMap#filter is wrong.#10381
[SPARK-12424][ML] The implementation of ParamMap#filter is wrong.#10381sarutak wants to merge 4 commits intoapache:masterfrom
Conversation
There was a problem hiding this comment.
the indent seems to be broken
|
LGTM except one minor comment. |
|
Test build #48008 has finished for PR 10381 at commit
|
|
Test build #48010 has finished for PR 10381 at commit
|
There was a problem hiding this comment.
LGTM. This could be on one line but I wouldn't change it
|
Test build #48019 has finished for PR 10381 at commit
|
|
cc: @mengxr @jkbradley |
|
LGTM |
There was a problem hiding this comment.
Hasn't this changed the logic slightly? now you compare to parent.uid
There was a problem hiding this comment.
I think the original logic is wrong because the type of .parent is String (this is a member of Param) while the type of parameter parent is Params.
According to the implementation of Param, the member parent of Param is passed uid of Identifiable which is a trait of Params.
class Param[T](val parent: String, val name: String, val doc: String, val isValid: T => Boolean)
extends Serializable {
def this(parent: Identifiable, name: String, doc: String, isValid: T => Boolean) =
this(parent.uid, name, doc, isValid)
|
Test build #48364 has finished for PR 10381 at commit
|
|
Thanks @srowen , @BenFradet and @yanboliang for the review. I'm merging this into |
ParamMap#filter uses `mutable.Map#filterKeys`. The return type of `filterKey` is collection.Map, not mutable.Map but the result is casted to mutable.Map using `asInstanceOf` so we get `ClassCastException`. Also, the return type of Map#filterKeys is not Serializable. It's the issue of Scala (https://issues.scala-lang.org/browse/SI-6654). Author: Kousuke Saruta <sarutak@oss.nttdata.co.jp> Closes #10381 from sarutak/SPARK-12424. (cherry picked from commit 07165ca) Signed-off-by: Kousuke Saruta <sarutak@oss.nttdata.co.jp>
ParamMap#filter uses
mutable.Map#filterKeys. The return type offilterKeyis collection.Map, not mutable.Map but the result is casted to mutable.Map usingasInstanceOfso we getClassCastException.Also, the return type of Map#filterKeys is not Serializable. It's the issue of Scala (https://issues.scala-lang.org/browse/SI-6654).